Skip to content

fix(cli): os explain query teaches where / orderBy, and the catalog sweep asserts key retention - #17263

Merged
os-project-manager merged 2 commits into
mainfrom
claude/issue-16925-explain-query-where-orderby
Sep 10, 2026
Merged

fix(cli): os explain query teaches where / orderBy, and the catalog sweep asserts key retention#17263
os-project-manager merged 2 commits into
mainfrom
claude/issue-16925-explain-query-where-orderby

Conversation

@claude

@claude claude Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Fixes #16925

Clause-②: no

os explain query documented two keys QuerySchema does not have. Both faces of
the entry now read the schema's own spellings, and the #14811 catalog sweep grows
the assertion that can actually see this failure mode.

The defect, measured at origin/main

BaseQuerySchema (packages/spec/src/data/query.zod.ts) is a plain z.object,
so an unknown top-level key is dropped, not refused. The shipped example was
therefore green through the sweep and empty in the result:

declared example keys: [ 'object', 'fields', 'filters', 'sort', 'limit' ]
parses: true
output keys:           [ 'object', 'fields', 'limit' ]
DROPPED:               [ 'filters', 'sort' ]

An author who copied it got a query that parsed clean and ran with no filter and
no ordering — with limit: 50 sitting next to it, a different set of rows under
an ordinary success, and nothing in the output saying so.

Half 1 — the entry, both faces

The real shapes were read off BaseQuerySchema directly, not inferred from the
old Filter[] / SortConfig[] type names:

  • where takes FilterConditionSchema — one condition tree, not an
    array. A field-keyed entry is a condition on that field (a bare value is
    implicit equality, an object is a map of $ operators), and $and / $or /
    $not combine conditions.
  • orderBy takes SortNodeSchema[], whose direction key is spelled
    order. direction is not a synonym — SortNodeSchema is strict and
    refuses it by name, because when it was merely stripped, order fell back to
    its asc default and a descending request came back ascending.

Both the example and the two optional-table rows were corrected. Verified by
parse, on the corrected literal:

parses: true
output keys: [ 'object', 'fields', 'where', 'orderBy', 'limit' ]  DROPPED: []
orderBy roundtrip: [{"field":"created_at","order":"desc"}]

Half 2 — the sweep grows a key-retention assertion

The #14811 sweep asserts safeParse(...).success === true, and a silent strip
keeps that true — the green over this entry was false by construction. The
new assertion is the one that can see it: an example must parse and come back
with every key it declares.

It lands as a ratchet, not as a patch over a large hole. All nine BOUND
entries were re-measured against spec for this PR by feeding each schema an
unknown top-level key:

object     ObjectSchema       -> REJECTS unknown key
field      FieldSchema        -> REJECTS unknown key
view       ViewSchema         -> REJECTS unknown key
flow       FlowSchema         -> REJECTS unknown key
agent      AgentSchema        -> REJECTS unknown key
app        AppSchema          -> REJECTS unknown key
query      QuerySchema        -> SILENT-STRIP  (success: true, probe key retained: false)
dashboard  DashboardSchema    -> REJECTS unknown key
action     ActionSchema       -> REJECTS unknown key

Class size is 1 of 9, and the direction of travel is closing — the open top
level is declared deliberate and already owned (query.zod.ts: 「Deliberately NOT taken here: BaseQuerySchema's own top level stays non-strict. That is
#4001's to schedule.」). So the assertion is green across the whole catalog
the day it lands; what it defends is the day a bound entry resolves to an open
top level again.

It covers the EXAMPLE face only, deliberately. evaluate(key) reads
catalog[key].example and has never read the optional / required tables —
and this card's two wrong spellings lived on both faces. Covering the table
face is not a stricter version of this assertion but a different one, and the
measurement says why: a table row is prose, not a key. Measured across the nine
entries, rows that are not a key of their schema:

view       rows-not-a-key: ["list | form | listViews | formViews"]   (prose, correct as written)
action     shape NOT INTROSPECTABLE through the actionObject() wrapper
every other entry (query included, after this PR): []

So a table-face assertion needs a way to tell a key name from a description, and
a second technique for the one entry whose shape it cannot read. That is a design
decision rather than a mechanical extension, so it is declared as a gap in the
test's own header and raised as an open question rather than half-built here.

Ablation — predictions written before any leg ran

Recorded in full before the first leg. Restore the pre-fix filters / sort
spellings in the example and re-run; the contrast is the whole card.

# Prediction Observed
P1 new retention assertion RED, naming the dropped keys RED — expected [ 'filters', 'sort' ] to deeply equal []
P2 existing safeParse assertion stays GREEN GREEN — ✓ os explain query — example parses as QuerySchema
P3 vitest exit non-zero, exactly 1 failing test Tests 1 failed, 39 passed of 40, wrapper exit 1
P4 restored: both green, exit 0 Tests 40 passed (40), wrapper exit 0
P5 restored: blob equals HEAD's, git diff HEAD empty equal, empty

No prediction came out wrong. The mutation was proven on disk before the run —
occurrence counts moved (where anchor 1 to 0, filters anchor 0 to 1) and the
blob hash moved off HEAD's 09ab4c72… to 6f8c85f5…; the restore was
git checkout HEAD -- ABSPATH (never bare), proven by blob equality,
git diff HEAD at 0 bytes and a clean git status --porcelain, all under a
trap ... EXIT INT TERM with absolute paths. No build leg is involved: the test
imports ../src/commands/explain by relative path, so the mutation reaches the
run from source.

Verification

  • pnpm --filter @objectstack/cli exec vitest run --project unit190 files,
    2644 passed
    . The integration tier is declared to CI: this diff touches no
    spawn entry, no bin/, and no test/helpers/serve-process.ts.
  • pnpm --filter '@objectstack/cli^...' build — dependency closure green;
    pnpm --filter @objectstack/cli build green (two published-subpath pins read
    the package's own dist/ and exit loudly without it — a prerequisite, not a
    finding).
  • pnpm --filter @objectstack/cli typecheck — reported below.
  • Gate family derived on the final tree with
    node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack
    and reconciled with --ran: 61 commands, 58 exit 0. Every exit code was
    captured by redirect-then-$?, never through a pipe.

Docs-drift round

Run on the final head. scripts/docs-audit/affected-docs.mjs named 15 candidate
pages; none carries the wrong spelling at query level — the hand-written docs were
already correct and os explain was the outlier (content/docs/data-modeling/queries.mdx:22-23
is the corrected shape verbatim). The four pages a prose-class sweep turns up for
filters: / sort: are all a different surface (a nav item's filters, a
single view's sort), and content/docs/ui/react-pages.mdx already documents the
silent-drop behaviour this change makes more true, correctly, with no edit owed.
Positive control for the sweep pattern: 2 hits in content/docs/ui/views.mdx and 2
in explain.ts at origin/main. Nothing under docs/ (which the tool never walks)
carries the shape either. No page edited; ⛔ nothing under content/docs/releases/**
was touched.

Acceptance notes / 验收备注

Noted, not filed — none of these is a reproducible defect, a contract violation, or
an authoring trap:

  • Triage's parenthetical about view's optional table does not hold on today's
    tree.
    view's optional rows are name / label / object and its required
    row is the slot row — no filters / sort rows. The two spellings appear in
    view's prose (a line comment and the container description), where they
    correctly describe a single view's keys one level down. The fence was honoured
    regardless: view is untouched, and its example remains [finding] os explain view's example teaches a flat view literal — ViewSchema is a CONTAINER (list / form / listViews / formViews) #15171's.
  • Inside that same fenced prose, the line comment spells filters where the entry
    description spells filter. An observation about a comment on another card's
    entry — not filed, not edited.
  • query's optional table lists six of QuerySchema's seventeen keys — search,
    searchFields, top, aggregations, groupBy, having and expand are absent.
    Missing members are an omission, not an error, and out of this card's scope.
  • Three gates in the derived family exited 3 (PREREQUISITE NOT MET) on the first
    sweep, none of them a finding; re-run after a full build, reported in the dev report.

Fences honoured


Generated by Claude Code

…rt key retention

The `query` catalog entry taught `filters` and `sort`. `BaseQuerySchema` is a
plain `z.object`, so both were dropped silently: the example parsed clean and
came back with no filter and no ordering, and the #14811 sweep was green on it
because `safeParse(...).success` stays true through a silent strip.

Both faces corrected to the schema's own spellings — `where` (one condition
tree, not a `Filter[]`) and `orderBy` (sort nodes whose direction key is
`order`, never `direction`).

The sweep grows a key-retention assertion: an example must parse AND survive
the parse with every declared key intact. Green across all nine bound entries
today — eight refuse unknown keys outright and `query` was the only open top
level — so it lands as a ratchet against a bound entry resolving to an open
top level again, not as a patch over a large hole.

Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/cli, touching 4 documentable anchor(s).

13 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/client-sdk.mdx (via orderBy (symbol, a field of const object SCHEMAS), orderBy (literal, a string literal in query))
  • content/docs/api/data-api.mdx (via orderBy (symbol, a field of const object SCHEMAS), orderBy (literal, a string literal in query))
  • content/docs/api/error-catalog.mdx (via orderBy (symbol, a field of const object SCHEMAS), orderBy (literal, a string literal in query))
  • content/docs/api/wire-format.mdx (via orderBy (symbol, a field of const object SCHEMAS), orderBy (literal, a string literal in query))
  • content/docs/automation/flows.mdx (via orderBy (symbol, a field of const object SCHEMAS), orderBy (literal, a string literal in query))
  • content/docs/data-modeling/queries.mdx (via orderBy (symbol, a field of const object SCHEMAS), orderBy (literal, a string literal in query))
  • content/docs/deployment/troubleshooting.mdx (via orderBy (symbol, a field of const object SCHEMAS), orderBy (literal, a string literal in query))
  • content/docs/getting-started/common-patterns.mdx (via orderBy (symbol, a field of const object SCHEMAS), orderBy (literal, a string literal in query))
  • content/docs/kernel/contracts/data-engine.mdx (via orderBy (symbol, a field of const object SCHEMAS), orderBy (literal, a string literal in query))
  • content/docs/kernel/runtime-services/data-service.mdx (via orderBy (symbol, a field of const object SCHEMAS), orderBy (literal, a string literal in query))
  • content/docs/protocol/kernel/http-protocol.mdx (via orderBy (symbol, a field of const object SCHEMAS), orderBy (literal, a string literal in query))
  • content/docs/protocol/objectql/index.mdx (via orderBy (symbol, a field of const object SCHEMAS), orderBy (literal, a string literal in query))
  • content/docs/protocol/objectql/query-syntax.mdx (via orderBy (symbol, a field of const object SCHEMAS), orderBy (literal, a string literal in query))

2 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v16.mdx (via os explain (command, read off packages/cli/src/commands/explain.ts))
  • content/docs/releases/v17/17-0.mdx (via orderBy (symbol, a field of const object SCHEMAS), orderBy (literal, a string literal in query))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: created_at (literal, 34 pages)
  • 4 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 23 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 470746ae4511db2f7c754d8aa6c247c3ce2120c4packageMentionDocs.

Which tree this was computed on

This run read content/docs from a9bc6955288edaa28b4a427e8fbace8130642cd5 — the merge of head 582bf0dd5fddfdcb8afb77111db0b87b38c7f26a into base 470746ae4511db2f7c754d8aa6c247c3ce2120c4, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin a9bc6955288edaa28b4a427e8fbace8130642cd5 && git checkout a9bc6955288edaa28b4a427e8fbace8130642cd5
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 470746ae4511db2f7c754d8aa6c247c3ce2120c4 582bf0dd5fddfdcb8afb77111db0b87b38c7f26a && git checkout -B drift-repro 470746ae4511db2f7c754d8aa6c247c3ce2120c4 && git merge --no-ff 582bf0dd5fddfdcb8afb77111db0b87b38c7f26a

node scripts/docs-audit/affected-docs.mjs --json 470746ae4511db2f7c754d8aa6c247c3ce2120c4

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 470746ae4511db2f7c754d8aa6c247c3ce2120c4 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Copy link
Copy Markdown
Collaborator

ACCEPT — armed for the queue

Dispatching seat (domain:cli execution PM, seat post #6024). Head 582bf0dd5fddfdcb8afb77111db0b87b38c7f26a, ready flip then auto-merge, in that order.

Pre-checks, each read rather than taken from the delivery report:

# check reading
Clause-② no, and it holds on the diff: no schema moved, no accept set moved, no published export added. --pair 17263 exit 0 per the dev. ⇒ no CONTRACT_REVIEW_TIER gate on the enqueue.
CI on head 33 check runs, 0 red, 0 in progress. Lint & Repo Gates success 01:40:36Z, TypeScript Type Check, Test Core rollup, Dogfood Regression Gate, Build Core, Temporal Conformance, Governed Surface Queue Guard all success. Three skipped are path-filtered or opt-in (Console Pin Gate, Build Docs, Packed-tarball smoke).
fences Read off the diff, not the report. 3 files, all packages/cli plus the changeset. ⛔ packages/spec untouched — the class root is #4001's. ⛔ the view catalog entry untouched — that neighbourhood is #15171's.
the two traps named at dispatch Both avoided. where is spelled as one condition tree (FilterCondition), not a Filter[]; orderBy's direction key is spelled order, not direction. Getting either wrong would have reproduced this card's own defect in the fix.
governed surface none.

⭐ What makes the new assertion worth its line. The pre-existing sweep asserted safeParse(...).success === true, which stays green over a key a non-strict z.object silently strips — it was green on this very defect on every run. The retention assertion asks the second question (parses and comes back whole), and the ablation showed the discrimination rather than a blanket red: with the old spellings restored, the new case fails naming [ filters, sort ] while the existing case stays green.

It lands as a ratchet, not a patch over a hole: all nine bound entries were read back to spec, eight refuse an unknown key outright and query alone had an open top level — deliberately, and already owned by #4001. So it is green across the catalog the day it lands, and what it defends is a reversal of that direction of travel.

Residue, filed rather than carried: the retention assertion reads the example face only, and the optional/required table rows carry key names too — the same authoring trap, unguarded, and half of this card's own defect lived there. Filed as #17266 with both obstacles the delivery measured (a table row is prose, not a key; one entry of nine is not introspectable through its wrapper) and the fences it inherits.

⚠️ Two triage readings corrected by the delivery, recorded so they are not rediscovered: view's optional table does not list filters / sort on today's tree — those spellings appear only in view's prose, where they correctly describe a single view's keys one level down. Both corrections and the prose's own filters / filter inconsistency belong to #15171, which owns that entry.


Generated by Claude Code

@os-project-manager
os-project-manager added this pull request to the merge queue Sep 10, 2026
Merged via the queue into main with commit 3a2d2b5 Sep 10, 2026
35 checks passed
@os-project-manager
os-project-manager deleted the claude/issue-16925-explain-query-where-orderby branch September 10, 2026 02:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[finding] os explain query's example teaches two keys QuerySchema SILENTLY DROPS — filters/sort are where/orderBy, and the #14811 sweep is green on it

2 participants